fix(solver) resolve a data race with ops counters #381
Conversation
Codecov Report
@@ Coverage Diff @@
## main #381 +/- ##
==========================================
- Coverage 54.19% 46.68% -7.52%
==========================================
Files 61 87 +26
Lines 4971 6568 +1597
==========================================
+ Hits 2694 3066 +372
- Misses 1978 3136 +1158
- Partials 299 366 +67
Continue to review full report at Codecov.
|
|
Had a conversation with @mflendrich on this and updated the code to use a mutex instead of atomic as that package has a ton of gotchas. |
solver/solver.go
Outdated
| a.counter += delta | ||
| } | ||
|
|
||
| func (a *AtomicInt32Counter) Read() int32 { |
There was a problem hiding this comment.
nit - Count might be a better name because it describes the returned value better, and because Read is already in use by io.Reader.
| for i := 0; i < 10; i++ { | ||
| go func() { | ||
| defer wg.Done() | ||
| a.Increment(int32(1)) | ||
| }() | ||
| } |
There was a problem hiding this comment.
optional: too see any meaningful risk of race condition occuring, you probably need each of the goroutines to increment the counter 1000 times in a loop.
There was a problem hiding this comment.
This is really to trigger a data race bug with go test -race.
If we disable locking, this reliably produces the race:
go test -race
==================
WARNING: DATA RACE
Read at 0x00c000156240 by goroutine 9:
github.com/kong/deck/solver.(*AtomicInt32Counter).Increment()
/home/hbagdi/deck/solver/solver.go:31 +0x6f
github.com/kong/deck/solver.TestStats_CreateOps.func1()
/home/hbagdi/deck/solver/solver_test.go:18 +0x66
Previous write at 0x00c000156240 by goroutine 8:
github.com/kong/deck/solver.(*AtomicInt32Counter).Increment()
/home/hbagdi/deck/solver/solver.go:31 +0x84
github.com/kong/deck/solver.TestStats_CreateOps.func1()
/home/hbagdi/deck/solver/solver_test.go:18 +0x66
Goroutine 9 (running) created at:
github.com/kong/deck/solver.TestStats_CreateOps()
/home/hbagdi/deck/solver/solver_test.go:16 +0xf2
testing.tRunner()
/usr/local/go/src/testing/testing.go:1194 +0x202
Goroutine 8 (finished) created at:
github.com/kong/deck/solver.TestStats_CreateOps()
/home/hbagdi/deck/solver/solver_test.go:16 +0xf2
testing.tRunner()
/usr/local/go/src/testing/testing.go:1194 +0x202
==================
--- FAIL: TestStats_CreateOps (0.00s)
testing.go:1093: race detected during execution of test
FAIL
exit status 1
FAIL github.com/kong/deck/solver 0.008s
No description provided.